Add canonicalizations for tracking-* utilities#19827
Conversation
Right now, when we _know_ that a replacement utility exist, then we use it immediately. However, if multiple match then we don't really know what to do so up until now we just bailed. But the funny part is that `-tracking-wide` and `tracking-tight` result in the exact same signature. In this situation, we pick the positive value instead of the negative value. If at any point, multiple positive utilities exist, we bail again. So this only covers the above scenario where a negative and positive utility was present.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds tests that verify canonicalization of 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/tailwindcss/src/canonicalize-candidates.test.ts (1)
1125-1156: Add a regression for the still-ambiguous branch.These cases cover the new “one positive + negatives” path, but not the early return in
packages/tailwindcss/src/canonicalize-candidates.ts, Lines 1138-1140 and Lines 1381-1383. A small fixture with two positive aliases sharing a signature and one negative alias would keep thefoo / -bar / bazbail-out behavior from regressing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tailwindcss/src/canonicalize-candidates.test.ts` around lines 1125 - 1156, Add a test to cover the ambiguous early-return branch in canonicalize-candidates: create a small fixture (using expectCanonicalization) that defines two positive aliases that share the same signature and one negative alias (e.g. the described "foo / -bar / baz" pattern) so the code path that bails out early in canonicalize-candidates.ts (the early return handling the ambiguous positive+negative mix) is exercised; place it alongside the existing tracking tests in canonicalize-candidates.test.ts and assert the candidate canonicalizes to the expected bail-out result to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/tailwindcss/src/canonicalize-candidates.test.ts`:
- Around line 1125-1156: Add a test to cover the ambiguous early-return branch
in canonicalize-candidates: create a small fixture (using
expectCanonicalization) that defines two positive aliases that share the same
signature and one negative alias (e.g. the described "foo / -bar / baz" pattern)
so the code path that bails out early in canonicalize-candidates.ts (the early
return handling the ambiguous positive+negative mix) is exercised; place it
alongside the existing tracking tests in canonicalize-candidates.test.ts and
assert the candidate canonicalizes to the expected bail-out result to prevent
regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3da09340-db94-4b25-8db3-4d558068fa80
📒 Files selected for processing (2)
packages/tailwindcss/src/canonicalize-candidates.test.tspackages/tailwindcss/src/canonicalize-candidates.ts
This PR adds support for canonicalizations for
tracking-*utilities.This one is a bit of a funny one, if you take a look at the linked issue, there is a beautiful table:
It doesn't really make sense to why only the
tracking-widestone is properly suggested here. Until you look a little bit closer.Turns out that
-tracking-tighteris equivalent totracking-wider,-tracking-tightis equivalent totracking-wideand so on.The way the canonicalization works internally is by generating a signature for a given utility class. If two utilities have the exact same signature, we can consider them the same. In this case
tracking-widestandtracking-[0.1em]have the same signature.One of the rules we have internally is that if we find more than one replacement utility then we don't really know what to do, so we bail. Because if you get
fooorbar, which one do you pick?If we refer to this above table again, the moment we want to canonicalize the
tracking-[-0.05em]we get two suggestions:tracking-tighterand-tracking-wider, since we don't know what to do, we bail and we don't suggest anything.So the reason that
tracking-widestwas suggested is just because we don't have a-tracking-tightest.How do we fix this? Well, since we have
tracking-*and-tracking-*utilities, I wanted to deprecate the-tracking-*ones for named utilities (where the values come from your theme) because that doesn't really make sense.However, we have this exact pattern documented here: https://tailwindcss.com/docs/letter-spacing#using-negative-values Which means that I can't just deprecate those utilities.
Instead, I added a different rule which says that if you get multiple possible replacements, then we prefer the "positive" one, the one without the
-. Also added some additional checks to make sure that if you getfoo,-bar,baz, that we also bail because we know that we should preferfooorbazover-bar, but we don't know if we should pickfooorbaz...This additional rule does solve the original issue, and we already prefer possible values over negative values in other places (related to bare values).
Fixes: tailwindlabs/tailwindcss-intellisense#1558
Test plan